Improves search to handle smaller search terms.
#4735
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4735 +/- ##
===========================================
+ Coverage 99.22% 99.24% +0.01%
===========================================
Files 303 303
Lines 23070 23262 +192
===========================================
+ Hits 22891 23086 +195
+ Misses 179 176 -3 ☔ View full report in Codecov by Sentry. |
|
Could I get a review on this one? |
|
@medha-14 Sorry, there is a bit of a backlog due to the upcoming release and everyone coming back from vacation. Don't worry, we will review this shortly |
|
Thank you for the update! I just thought you missed this one. |
There was a problem hiding this comment.
Thanks! I think min_similarity could be slightly higher here because the threshold here is too low and can lead to false positives in the search. But, at the same time, being able to resolve potential typos in the search query would require a lower threshold. This makes me feel that we could expose it with a sensible default, but I don't yet know what a sensible default would be. It should be higher than the current 40%, though. In-line comment about this below:
| @@ -163,14 +185,24 @@ def search(self, keys: str | list[str], print_values: bool = False): | |||
| search_keys = [k.strip().lower() for k in keys if k.strip()] | |||
|
|
|||
| known_keys = list(self.keys()) | |||
| known_keys.sort() | |||
|
|
|||
| min_similarity = 0.4 | |||
There was a problem hiding this comment.
min_similarity is also defined above, so it gets defined twice. How about making it an argument for _find_matches()?
Follow-up question: do you think it would make sense to expose it publicly for users through search() as well?
Sorry for the delayed response. I think it's important to clarify that |
Description
Fixes #4734
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ pre-commit run(or$ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python -m pytest(or$ nox -s tests)$ python -m pytest --doctest-plus src(or$ nox -s doctests)You can run integration tests, unit tests, and doctests together at once, using
$ nox -s quick.Further checks: